-
Notifications
You must be signed in to change notification settings - Fork 133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ASoC: SOF: pcm: Improve debug and error prints #5252
base: topic/sof-dev
Are you sure you want to change the base?
ASoC: SOF: pcm: Improve debug and error prints #5252
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one @ujfalusi - do you think similar can be done for kcontrols
I'm going through the files and logs to see what can be improved. With a good log we can cut down on some back and forth to get the logs which have information and in case if it is a hard to reproduce issue, that can take a while... |
sound/soc/sof/sof-audio.h
Outdated
* The spcm_dbg() is prepared to handle a call without valid direction (dir == -1) | ||
*/ | ||
#define spcm_dbg(__spcm, __dir, __fmt, ...) { \ | ||
if (__dir == -1) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ujfalusi this is really used only at the time of creation. Do we really need to special handle it just for that? In all practical cases, I think we're going to have the dir defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is only in one case, I can put it as __unlikely(), printing the dir and a number would be confusing to look at if the direction is not valid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no what i was suggesting was that we leave out the new macro for this one case and use it everywhere else with a valid direction.
Move the sof_pcm_stream_free() from sof-audio.c to pcm.c as static function and add wrapper to free all active stream, which is going to be used in ipc3/4 topology code (removes duplicated code). With this change most of the PCM stream related code is located in one source file for easier lookup and simplified flow. Signed-off-by: Peter Ujfalusi <[email protected]>
… open The platform specific pcm_open call via snd_sof_pcm_platform_open() can modify the initial buffer configuration via constraints. Move the prints as last step in the sof_pcm_open() function to reflect the final setup. Signed-off-by: Peter Ujfalusi <[email protected]>
…ev_err() Introduce spcm_dbg() and spcm_err() macros to provide consistent printing for debug and error messages which includes usable information in the print's prefix. Update the prints in pcm.c, ipc3-pcm.c and ipc4-pcm.c to take advantage of the features provided by the macros. Signed-off-by: Peter Ujfalusi <[email protected]>
b3273cb
to
8097cfa
Compare
Changes since v1:
|
SOFCI TEST |
1 similar comment
SOFCI TEST |
The current results look as expected, I will rerun again though as some DUTs just got a BIOS update. |
SOFCI TEST |
Hi,
Our error printing alone leaves out information which can provide a needed context for the error itself.
For example in IPC3: "ipc tx error for 0x60010000 (msg/reply size: 108/20): -22"
This will not tell which PCM device, what direction this error had happened.
Similarly the debug prints are using ad-hoc formatting and they are not consistent, one needs to look for prior or later prints for context, which can be problematic with concurrent PCM operation.
At the heart this series will switch to a spcm_dbg() and spcm_err() wrapper to include useful information in prints where spcm is available.
As preparation, the PCM functions are relocated to pcm.c from sof-audio.c.